Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixes invalid monitor range bug & adds range validation #17

Merged
merged 4 commits into from
Apr 28, 2015

Conversation

baileyparker
Copy link
Contributor

Fixes #16.

Monitor now displays a warning message about an invalid range instead of hanging browser.

Also adds basic validation to monitor start/length inputs. If a negative value is entered for either or if start + length is beyond the 6502's 64 KiB of memory, the fields will turn red to indicate the problem.

@BigEd
Copy link
Collaborator

BigEd commented Apr 26, 2015

Please could you fix some little bugs in this request?

  • monitor is mis-spelt
  • some possible out-by-one bugs in the comparisons against top of memory
  • please always use 0xffff in preference to using 65536 (or 65535)

@baileyparker
Copy link
Contributor Author

I've fixed 1 & 3. However, I don't see what you mean by "one-off errors in the comparisons against the top of memory". From the easy6502 page:

Thee 6502 uses a 16-bit address bus, meaning that there are 65536 bytes of memory available to the processor. Remember that a byte is represented by two hex characters, so the memory locations are generally represented as $0000 - $ffff. There are various ways to refer to these memory locations, as detailed below.

The reason why I'm using 0x10000 is because length does not start from zero. For example, you could only want the monitor to display the very last word in memory. To do so you'd specify a start of $ffff and a length of 1. If the upper limit was 0xffff then start + length would exceed it, even though this is a valid range.

@BigEd
Copy link
Collaborator

BigEd commented Apr 27, 2015

Oops, you are quite right - it was my own out by one error. Noting that I have seen your request to use start and end instead of start and length, I wonder if it would be clearer to use
(start + length - 1) <= 0xffff
If you don't agree, just let me know and I'll merge as-is.
If you do agree, please make the corresponding tweaks, and I'll merge.
Thanks!

@baileyparker
Copy link
Contributor Author

I agree. I've made the changes. I think this way (the way you proposed), is a lot easier to think about.

BigEd added a commit that referenced this pull request Apr 28, 2015
Fixes invalid monitor range bug & adds range validation
@BigEd BigEd merged commit adf10c8 into skilldrick:gh-pages Apr 28, 2015
@BigEd
Copy link
Collaborator

BigEd commented Apr 28, 2015

That's great - merged. I'm trying to keep this (original) fork as stable as possible, in a strict maintenance-only mode. I intend to put a note into the page and the readme to promote any other actively-developed forks. The idea is that anyone landing at http://skilldrick.github.io/easy6502/ will have something worthwhile to use, and also have pointers to more advanced developments. The various active forks will of course be free to share their innovations amongst themselves, and no-one need be held up by my caution or inattention.

@baileyparker baileyparker deleted the fix_monitor_browser_hang branch April 28, 2015 20:29
@baileyparker
Copy link
Contributor Author

So you recommend moving Issues #11 #13 #18 and #19 into my own fork instead trying to PR them here?

@BigEd
Copy link
Collaborator

BigEd commented Apr 28, 2015

#11 I'd like to get fixed, but yes please, let's regard the others as enhancements to your fork.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Monitor hangs browser for addresses beyond $ffff with no warning
2 participants